-
Notifications
You must be signed in to change notification settings - Fork 875
[WIP] chore(ci): add check-deploy to CI #1867
Conversation
@@ -7,3 +7,4 @@ npm install --no-optional | |||
(cd public/docs/_examples/_protractor && npm install --no-optional) | |||
npm run webdriver:update --prefix public/docs/_examples/_protractor | |||
gulp add-example-boilerplate | |||
sh -e /etc/init.d/xvfb start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem with this is that I couldn't use this script anymore on my machine to install the repo, right? Couldn't we leave this on the travis file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, didn't know that was an issue but it makes sense.
581e275
to
fbfb759
Compare
This PR will require that the latest release be updated on the |
4dc91f6
to
80374f6
Compare
@chalin can you review and tell me if this is what you intended? I'm talking with @naomiblack for a way around the |
80374f6
to
c2c1437
Compare
Great to see this move forward! Please see my comments below. The gulp.task('check', ['build-docs'], function() {
return harpCompile();
}); Note that Actually, I think that we can simplify the situation by having a single env:
...
matrix:
- TASK=lint
- TASK='e2e --fast' PREVIEW=false
- TASK='e2e --fast' PREVIEW=true
- TASK='check' PREVIEW=false
- TASK='check' PREVIEW=true
matrix:
...
allow_failures:
- env: "TASK='e2e --fast' PREVIEW=true"
- env: "TASK='check' PREVIEW=true"
...
install:
- ./scripts/install.sh Then Btw, eventually we may also want to add checks on the output produced by the harp compile command to help determine if the Looking forward to having this in master. |
@chalin thanks for the feedback! I think the simpler task for harp seems like a good idea, I'll put that in. I don't think having a simple script with conditionals inside is a good idea though. It might default to what That is why I renamed it to I see the CI being expanded to also running dart if possible, but even in that case it's still useful to have the standalone scripts that setup your environment for what you need instead of having to setting flags locally to guide an install script. |
Ah, I remember why I left I'll update the readme to reflect that. |
c2c1437
to
9e80e41
Compare
Updated the PR with a dedicated gulp task and updated readme. |
LGTM |
@naomiblack this PR still missing the cc @IgorMinar |
Merging because @chalin and @filipesilva say so. Presumably they'll fix a busted CI. |
This PR adds CI entries to the existing matrix that run
gulp check-deploy
for the latest release and for the master branch of angular.io.